[Security] Correcting bugs for provider and JSON login #16799
Conversation
|
I think this proposal is correct, but let's ask @wouterj to double check. Thanks. |
carsonbot
changed the title
|
@KosticDusan4D thanks again for this contribution. Sorry for the late merge, but I prefer to merge this until someone who knows security well reviews it. Thanks for your patience 🙏 |
|
The configuration change looks good, but the controller part does not. The current signature is correct, we do want a type declaration. Here |
On lines 501 and 502 I propose this change, because it confused me so much which provider to use, and in the case that you are explaining there you should use earlier configured app_user_provider, not users_in_memory. On line 1043 I had a problem that $user is always null until I deleted the "?User" from parameters. I think that PHP gets confused here with the way it is written. And to say that now with that part removed everything works fine. Thanks in advance and I hope that I contributed even a little as this is.
|
Hi @KosticDusan4D! Thanks for this contribution. Fixing a bug in the code example is a great way to contribute, it'll safe upcoming readers of the docs hours of debugging :) I've reverted the change in the code example, the |
On lines 501 and 502 I propose this change, because it confused me so much which provider to use, and in the case that you are explaining there you should use earlier configured app_user_provider, not users_in_memory.
On line 1043 I had a problem that $user is always null until I deleted the "?User" from parameters. I think that PHP gets confused here with the way it is written. And to say that now with that part removed everything works fine.
Thanks in advance and I hope that I contributed even a little as this is.